Hotfix: restore BoundsForRange gate, raise focus poll to 80ms#255
Merged
Conversation
PR #218 made the BoundsForRange AX call unconditional in three places — including the deep-tree walker that visits many AXStaticText leaves per focus poll. BoundsForRange is a synchronous cross-process call into the focused app's AX implementation; in Chrome that's a round-trip into the renderer, and calling it on nodes that don't advertise support stalled the main thread badly enough to freeze typing. Restore the supportsBoundsForRange gate at the three call sites and keep the rectIsNearAnchor validator for elements that do advertise support. Bump the first-launch focus poll interval 50ms -> 80ms and floor the persisted value at the shipped default so existing installs migrate (the stepper is hidden from the UI, so the persisted value is always the previous default, never a user-chosen override).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #218 made the BoundsForRange AX call unconditional, including inside the deep-tree walker that visits many
AXStaticTextleaves per focus poll.BoundsForRangeis a synchronous cross-process call into the focused app's AX implementation — in Chrome that's a round-trip into the renderer, and calling it on nodes that don't advertise support stalled the main thread badly enough to freeze typing (users reported the menu bar showing the loading indicator while typing in Chrome text fields). This restores thesupportsBoundsForRangegate at the three call sites while keeping #218'srectIsNearAnchorvalidator for elements that do advertise support. Also bumps the focus poll interval 50ms → 80ms to reduce baseline AX pressure on the main thread.Validation
test-without-buildingfails locally with a Team ID / code-signing mismatch on the host app vs. test bundle (code signature ... not valid for use in process: mapping process and mapped file (non-platform) have different Team IDs) — same condition called out inCLAUDE.md. Tests need to be run in CI or with a configured signing identity to verify end-to-end. TheAXTextGeometryResolverTestswere updated to passsupportsBoundsForRange: truefor the restored parameter; therectIsNearAnchorunit tests are unchanged and still cover the anchor accept/reject boundary.Manual smoke test pending — should be verified by typing in a Chrome text field on a page with a large AX tree (Gmail, Google Docs, GitHub PR review) before promoting.
Linked issues
Refs #218
Risk / rollout notes
BoundsForRangewithout advertising it inparameterizedAttributeNames(a minority of Electron/WebKit cases referenced in Try BoundsForRange optimistically for exact caret placement #218's commit message) will fall back toTextMarker→ child text runs →AXFrameinstead of optimisticBoundsForRange. This is the pre-Try BoundsForRange optimistically for exact caret placement #218 behavior — overlay placement on those elements regresses to where it was before that PR, which was acceptable enough to ship for months.cotabbyFocusPollIntervalMilliseconds = 50persisted inUserDefaults. The resolver now floors the persisted value at the shipped default, so the 50ms → 80ms bump reaches existing users. The setting stepper is hidden from the UI (SettingsView.swift:646), so the persisted value is always the previous first-launch default, never a user-chosen override — flooring is safe.parameterizedAttributeNamesper AX element for the duration of a focus session so we can keep the gate cheap while still picking up genuine supporters that the system later starts to advertise.